Skip to content

Popover: focus management for dialog role - #34413

Merged
dmlvr merged 19 commits into
DevExpress:feature/26_2_popover_support_content_readingfrom
dmlvr:dmlvr-feature/26_2_popover_support_content_reading
Jul 29, 2026
Merged

Popover: focus management for dialog role#34413
dmlvr merged 19 commits into
DevExpress:feature/26_2_popover_support_content_readingfrom
dmlvr:dmlvr-feature/26_2_popover_support_content_reading

Conversation

@dmlvr

@dmlvr dmlvr commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@dmlvr dmlvr self-assigned this Jul 21, 2026
@dmlvr dmlvr added the 26_2 label Jul 21, 2026
@dmlvr dmlvr changed the title Dmlvr feature/26 2 popover support content reading Popover: support content reading on opening Jul 21, 2026
@dmlvr dmlvr changed the title Popover: support content reading on opening Popover: move focus to role dialog and return to target, add default localization role when it doesn't specified Jul 21, 2026
…ontent_reading' into dmlvr-feature/26_2_popover_support_content_reading
@dmlvr
dmlvr force-pushed the dmlvr-feature/26_2_popover_support_content_reading branch from 0e86e91 to 8010fa2 Compare July 22, 2026 11:07
@dmlvr dmlvr changed the title Popover: move focus to role dialog and return to target, add default localization role when it doesn't specified Popover: move focus to role dialog and return to target Jul 22, 2026
@dmlvr
dmlvr force-pushed the dmlvr-feature/26_2_popover_support_content_reading branch from 7242766 to a17511a Compare July 22, 2026 12:58
@dmlvr dmlvr changed the title Popover: move focus to role dialog and return to target Popover: focus management for dialog role Jul 22, 2026
@dmlvr
dmlvr force-pushed the dmlvr-feature/26_2_popover_support_content_reading branch 2 times, most recently from 55c6f73 to d7d5b55 Compare July 23, 2026 07:33
@dmlvr
dmlvr marked this pull request as ready for review July 23, 2026 08:12
@dmlvr
dmlvr requested a review from a team as a code owner July 23, 2026 08:12
Comment thread packages/devextreme/js/__internal/ui/popover/popover.ts Outdated
@dmlvr
dmlvr force-pushed the dmlvr-feature/26_2_popover_support_content_reading branch from d7d5b55 to 5f6187d Compare July 23, 2026 13:46
@dmlvr
dmlvr force-pushed the dmlvr-feature/26_2_popover_support_content_reading branch from 42cfdc2 to 5953dba Compare July 27, 2026 16:09
@dmlvr
dmlvr force-pushed the dmlvr-feature/26_2_popover_support_content_reading branch from d6ab547 to be5e944 Compare July 28, 2026 14:26
@dmlvr
dmlvr force-pushed the dmlvr-feature/26_2_popover_support_content_reading branch from 88ad727 to 42151cd Compare July 28, 2026 16:46
if (!$first) {
// @ts-expect-error is should can get function as callback
const isCurrentTabbable = $currentElement?.is(selectors.tabbable);
const isCurrentNotOverlay = $currentElement?.get(0) !== this._$content?.get(0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we include overlay selector in the previous one:

$currentElement
    .is(selectors.tabbable)
    .not(overlayId);

?

if (!$last) {
// @ts-expect-error is should can get function as callback
const isReverseTabbable = $reverseElement?.is(selectors.tabbable);
const isReverseNotOverlay = $reverseElement?.get(0) !== this._$content?.get(0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same comment about selector

}

_restoreTargetFocus(): void {
const $targets = this._getAriaDescriptionTargets();

@Raushen Raushen Jul 28, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can have only one target. We can apply changes from this comment.

}
}

_forceFocusLost(): void {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check base method. May we can move there this logic.
Also please check what exactly base method do and why.

@dmlvr dmlvr Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check base method. Also please check what exactly base method do and why.

The base Overlay._forceFocusLost() checks if the currently focused element is inside the overlay content (this._$content) and blurs (resets) it if so. This prevents focus from being trapped on hidden/invisible elements when the overlay closes, which would break keyboard navigation.

May we can move there this logic?

No. The base Overlay class lacks the role-awareness (_getEffectiveAriaRole()) and target resolution mechanics (_getAriaDescriptionTargets()) that Popover uses to identify and refocus its trigger element.

@@ -890,6 +938,10 @@ class Popover<
}

_dispose(): void {
const { visible } = this.option();
if (visible && this._getEffectiveAriaRole() === 'dialog') {
this._restoreTargetFocus();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we check if we have focus inside?

@@ -111,6 +111,8 @@ export interface PopoverProperties extends Omit<Properties,
_overlayContentRole?: string;

_describeTarget?: boolean;

_preventDialogFocus?: boolean;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about _preventDialogContainerFocus?

@Raushen
Raushen requested a review from a team July 28, 2026 19:14
@dmlvr dmlvr closed this Jul 29, 2026
@dmlvr dmlvr reopened this Jul 29, 2026
@dmlvr
dmlvr requested a review from a team as a code owner July 29, 2026 14:26
dmlvr and others added 3 commits July 29, 2026 17:27
…r-feature/26_2_popover_support_content_reading

Signed-off-by: Dmitry Lavrinovich <52966626+dmlvr@users.noreply.github.com>
…github.com:dmlvr/DevExtreme into dmlvr-feature/26_2_popover_support_content_reading
@dmlvr
dmlvr merged commit 8e0f3d5 into DevExpress:feature/26_2_popover_support_content_reading Jul 29, 2026
97 of 99 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants